Add test coverage for substitution edgecases involving E notation#824
Conversation
Signed-off-by: Daniel <cranston.daniel@gmail.com>
f4962a8 to
e11ae22
Compare
christophebedard
left a comment
There was a problem hiding this comment.
Looks good to me.
This is completely separate from ros2/launch_ros#384 and ros2/launch_ros#436, right? As in, the behaviour this is testing is correct?
|
Pulls: #824 |
|
Yes I would say it is separate.
If "correct" means "consistent with PYYAML 1.1", then yes I believe it's largely correct. I'm not an expert in the nuances of YAML specs, but from the discussion in ros2/launch_ros#384 (comment), I'm a bit on the fence on the If _permute_assertion('1234e1', '12340', lc, 'true') # Arguably wrong. PYYAML treats LHS as a string, so shouldn't equal RHS
_permute_assertion('1234E2', '123400', lc, 'true') # Arguably wrong. PYYAML treats LHS as a string, so shouldn't equal RHS
_permute_assertion('1234E2', '1234E2', lc, 'true') # OK, PYYAML treats the LHS as a string anyway
_permute_assertion("'1234E2'", '123400', lc, 'false') # OK, LHS is coerced to string
_permute_assertion("'1234E2'", '1234E2', lc, 'false') # OK, LHS is coerced to string
_permute_assertion("'1234E2'", "'1234E2'", lc, 'true') # OK I guess.Though I have a feeling that, for original authors, the measure of "correctness" isn't YAML 1.1 but something else (maybe some heuristics to reflect how ROS 1 It's also interesting that |
I extended some tests while doing some investigations in ros2/launch_ros#384 (comment). I figured it worthwhile to make a PR in case you find them useful.
I might be missing other tests that could get a similar treatment, let me know if that's the case!